-
Notifications
You must be signed in to change notification settings - Fork 1
[REFACTOR] 스터디 검색 API 관련 요구사항 반영 #440
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe internal logic of the Changes
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/example/spot/service/study/StudyQueryServiceImpl.java(1 hunks)src/test/java/com/example/spot/service/study/StudyQueryServiceTest.java(3 hunks)
👮 Files not reviewed due to content moderation or server errors (2)
- src/main/java/com/example/spot/service/study/StudyQueryServiceImpl.java
- src/test/java/com/example/spot/service/study/StudyQueryServiceTest.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_test
🔇 Additional comments (5)
src/main/java/com/example/spot/service/study/StudyQueryServiceImpl.java (2)
678-678: LGTM! Clean refactor to Spring Data JPA method.The change from
searchByTitletofindAllByTitleContainingsuccessfully implements the like-based query approach as intended. The Spring Data JPA naming convention will automatically generate the appropriate SQL LIKE query for partial title matching.
685-685: LGTM: Count method updated consistently.The count method has been properly updated to
countAllByTitleContainingwhich maintains consistency with the query method change and ensures accurate pagination.src/test/java/com/example/spot/service/study/StudyQueryServiceTest.java (3)
1569-1569: LGTM! Test properly updated to match service refactor.The test changes correctly update mocking and verification to use
findAllByTitleContaininginstead ofsearchByTitle. Both success and failure test scenarios are appropriately covered, ensuring the refactored search functionality remains well-tested.Also applies to: 1592-1592, 1602-1602
1569-1570: LGTM: Test mocks updated consistently with service changes.The test has been properly updated to mock and verify the new
findAllByTitleContainingmethod instead ofsearchByTitle. The test logic and assertions remain unchanged, which is appropriate since only the underlying repository method changed while the business logic stays the same.Also applies to: 1592-1592, 1602-1603
1569-1569: LGTM! Test mocks properly updated to match service implementation.The test file has been correctly updated to mock and verify the new repository method
findAllByTitleContaininginstead of the oldsearchByTitle. This ensures the tests remain valid and continue to verify the expected behavior.Also applies to: 1592-1592, 1602-1602
| String keyword, StudySortBy sortBy) { | ||
| // 키워드로 스터디 조회 | ||
| List<Study> studies = studyRepository.searchByTitle(keyword, sortBy, pageable); | ||
| List<Study> studies = studyRepository.findAllByTitleContaining(keyword, sortBy, pageable); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
LGTM: Consistent implementation of like-based search.
The change to findAllByTitleContaining properly implements the like-based query method as mentioned in the PR objectives. The Spring Data JPA naming convention correctly translates to a SQL LIKE '%keyword%' operation for more flexible title searching.
Let's verify that the old searchByTitle method is no longer referenced elsewhere in the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Check for any remaining references to the old searchByTitle method
# Expected: No occurrences of searchByTitle method calls
echo "Searching for any remaining references to searchByTitle method..."
rg "searchByTitle" --type javaLength of output: 516
🏁 Script executed:
#!/bin/bash
# Description: Check for any invocation sites of the old searchByTitle method
echo "Searching for any call sites to searchByTitle..."
rg "\.searchByTitle" -n --type javaLength of output: 144
Remove unused searchByTitle declarations
Our search confirms there are no remaining calls to searchByTitle, but its signatures still exist in the custom repository interface and implementation. These should be cleaned up to avoid dead code:
• src/main/java/com/example/spot/repository/querydsl/StudyRepositoryCustom.java
• src/main/java/com/example/spot/repository/querydsl/impl/StudyRepositoryCustomImpl.java
Please remove the searchByTitle(...) method from both files.
🤖 Prompt for AI Agents
In src/main/java/com/example/spot/repository/querydsl/StudyRepositoryCustom.java
and
src/main/java/com/example/spot/repository/querydsl/impl/StudyRepositoryCustomImpl.java,
remove the unused searchByTitle method declarations and implementations
respectively, as they are no longer called anywhere in the codebase. This
cleanup will eliminate dead code and improve maintainability.
💡 Verification agent
🧩 Analysis chain
LGTM! Improved method naming follows Spring Data JPA conventions.
The change from searchByTitle to findAllByTitleContaining provides better clarity about the method's functionality and follows Spring Data JPA query method naming conventions.
Verify that all references to the old searchByTitle method have been updated throughout the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Search for any remaining references to the old searchByTitle method
# Expected: No occurrences should be found
echo "Searching for old method name 'searchByTitle'..."
rg "searchByTitle" --type java
echo "Searching for new method name 'findAllByTitleContaining'..."
rg "findAllByTitleContaining" --type javaLength of output: 1803
Remove remaining searchByTitle methods in the custom repository
The search revealed that the old searchByTitle API still exists alongside the new findAllByTitleContaining in your QueryDSL-based repository. To complete the rename and avoid confusion, please remove or rename these leftover definitions:
• src/main/java/com/example/spot/repository/querydsl/StudyRepositoryCustom.java
- List<Study> searchByTitle(String keyword, StudySortBy sortBy, Pageable pageable);
+ // removed in favor of findAllByTitleContaining(...)• src/main/java/com/example/spot/repository/querydsl/impl/StudyRepositoryCustomImpl.java
- public List<Study> searchByTitle(String keyword, StudySortBy sortBy, Pageable pageable) {
- …implementation…
- }
+ // implementation consolidated under findAllByTitleContaining(...)After removing these, ensure any callers (if any) are updated to use findAllByTitleContaining.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/com/example/spot/service/study/StudyQueryServiceImpl.java at
line 678, you are already using the new method findAllByTitleContaining. To
complete the transition, remove the old searchByTitle method definitions from
src/main/java/com/example/spot/repository/querydsl/StudyRepositoryCustom.java
and
src/main/java/com/example/spot/repository/querydsl/impl/StudyRepositoryCustomImpl.java.
Also, verify that no other code calls searchByTitle; if found, update those
calls to use findAllByTitleContaining instead.
#️⃣ 연관된 이슈
🔎 작업 내용
📷 스크린샷 (선택)
💬리뷰 요구사항 (선택)
Summary by CodeRabbit
Bug Fixes
Tests